Skip to content

feat: configuration callback composition #149

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

feat: configuration callback composition #149

wants to merge 1 commit into from

Conversation

@JohannesLamberts
Copy link

@JohannesLamberts JohannesLamberts commented Jun 20, 2020 •
edited
Loading

In two projects I am currently working on there are often common configurations needed to install components or plugins - or to setup the router and store instances.

To integrate multiple configuration setups I found it beneficial to compose them automatically like this:

render(
   Component, 
   { routes: [ { path: '/email_confirm' } ] }, 
   [
      installGlobalComponents,
      installVueI18n, 
      goToRoute('/email_confirm?token=afhugo823'),
   ]
)

I hope the example gives an impression of how this can help one to reuse configurations between tests. This allows to reduce the code that needs to be written per tests and to avoid duplications.

What do you think of this?

Two more things :)

  • I removed the check if the configurationCb is a function as it is not actually covered and could lead to strange behaviour (a not-falsy value is passed, but there is no error). If you think this should not be removed I can revert the change or throw an explicit error (e.g. "TypeError: configurationCb should be a function or an array of functions, got typeof configurationCb")
  • This changes the signature of the function which requires an update for TypeScript type definitions. Would you be interested in merging the definitions inside this repo?

@codecov
Copy link

codecov bot commented Jun 20, 2020 •
edited
Loading

Codecov Report

Merging #149 into master will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##            master      #149   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            1         1           
  Lines           70        75    +5     
  Branches        13        14    +1     
=========================================
+ Hits            70        75    +5
Impacted Files Coverage Δ
src/vue-testing-library.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4c5f724...ba037c8. Read the comment docs.

@afontcu
Copy link
Member

afontcu commented Jun 21, 2020

Hi! 👋 Thanks for taking the time to put this together!

What I usually do is I create my "own" render method with the desired configuration, and then use it instead of the one coming from VTL. In the docs we have this example using React Testing Library, but the same idea still applies.

Would that suit your needs? I'd really like to stick to that pattern instead of increasing the API of the library, so that it stays simpler and more aligned with other libraries from the testing-library ecosystem.

We might want to document that Custom render method more prominently, though!

@JohannesLamberts
Copy link
Author

JohannesLamberts commented Jun 21, 2020

Hi @afontcu

thanks for your reaction :) I understand that you don't want to increase the API of the library, even though I will then need to copy/share the pattern with the composeConfigurationCallbacks manually between projects.

If you'd like I can change the MR to just adding examples. I'd then propose to add a shared-setup.js (naming ?) example in which:

  • a "test-utils.js" provides a customRender function (with changed API / default options) as well as the composeConfigurationCallbacks function
  • use cases can be shown for the customRender function
  • use cases can be shown for composition with the original render function

What do you think?

@afontcu
Copy link
Member

afontcu commented Jun 21, 2020

Sure! For the time being I'd feel more comfortable with adding some examples on how to customise the render method. I'd start with a basic example (something simple such as adding i18n to all tests, or a common dependency)

Thanks for this! 🙌

@afontcu afontcu added the wontfix This will not be worked on label Feb 17, 2021
@JohannesLamberts
Copy link
Author

JohannesLamberts commented Mar 13, 2021

completely forget about this MR 👀.

To me, wrapping the render method adds opacity, as it hides which dependencies the tested component actually has, but I'll finally close this MR ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

wontfix This will not be worked on

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@JohannesLamberts @afontcu